Subgraph io fixes#12281
Conversation
📝 WalkthroughWalkthroughAdds IO-node link flags, adjusts hover/hit-testing and pointer forwarding for subgraph canvases, emits subgraph-level slot-link events on connect/disconnect, refactors subgraph slot validity checks, and adds a Playwright test plus helper to compute a subgraph input node's canvas bounds. ChangesVue Subgraph Slot Link Interactions
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 1623 passed, 0 failed · 3 flaky📊 Browser Reports
|
📦 Bundle: 5.36 MB gzip 🔴 +334 BDetailsSummary
Category Glance App Entry Points — 26.1 kB (baseline 26.1 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.24 MB (baseline 1.24 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 82.9 kB (baseline 82.9 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed / 2 unchanged Panels & Settings — 527 kB (baseline 527 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed / 14 unchanged User & Accounts — 17.8 kB (baseline 17.8 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed / 2 unchanged Editors & Dialogs — 112 kB (baseline 112 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 4 added / 4 removed UI Components — 58 kB (baseline 58 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed / 8 unchanged Data & Services — 3.16 MB (baseline 3.16 MB) • 🔴 +739 BStores, services, APIs, and repositories
Status: 13 added / 13 removed / 4 unchanged Utilities & Hooks — 366 kB (baseline 366 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 13 added / 13 removed / 18 unchanged Vendor & Third-Party — 9.94 MB (baseline 9.94 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 9.16 MB (baseline 9.16 MB) • ⚪ 0 BBundles that do not match a named category
Status: 57 added / 57 removed / 86 unchanged ⚡ Performance Report
All metrics
Historical variance (last 15 runs)
Trend (last 15 commits on main)
Raw data{
"timestamp": "2026-05-15T02:38:32.240Z",
"gitSha": "25b92a89196fd2fbeb543811a3866b2651b9d506",
"branch": "austin/subgraphIO-fixes",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2034.165999999999,
"styleRecalcs": 6,
"styleRecalcDurationMs": 5.495999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 399.956,
"heapDeltaBytes": 23145456,
"heapUsedBytes": 72858148,
"domNodes": 12,
"jsHeapTotalBytes": 14680064,
"scriptDurationMs": 18.966,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-idle",
"durationMs": 2018.5089999999946,
"styleRecalcs": 8,
"styleRecalcDurationMs": 8.215,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 419.003,
"heapDeltaBytes": 23165612,
"heapUsedBytes": 71729940,
"domNodes": 16,
"jsHeapTotalBytes": 14680064,
"scriptDurationMs": 23.122,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1813.2760000000019,
"styleRecalcs": 74,
"styleRecalcDurationMs": 38.478,
"layouts": 12,
"layoutDurationMs": 3.9189999999999996,
"taskDurationMs": 772.566,
"heapDeltaBytes": 18729708,
"heapUsedBytes": 67952272,
"domNodes": 58,
"jsHeapTotalBytes": 15990784,
"scriptDurationMs": 129.84799999999998,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1963.0180000000337,
"styleRecalcs": 80,
"styleRecalcDurationMs": 53.43099999999999,
"layouts": 12,
"layoutDurationMs": 3.9639999999999995,
"taskDurationMs": 945.254,
"heapDeltaBytes": 14448688,
"heapUsedBytes": 63149472,
"domNodes": -258,
"jsHeapTotalBytes": 21622784,
"scriptDurationMs": 140.362,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1761.3769999999818,
"styleRecalcs": 32,
"styleRecalcDurationMs": 20.663999999999998,
"layouts": 6,
"layoutDurationMs": 0.7649999999999998,
"taskDurationMs": 339.5199999999999,
"heapDeltaBytes": 809760,
"heapUsedBytes": 50078920,
"domNodes": 79,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 30.029999999999994,
"eventListeners": 19,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "canvas-zoom-sweep",
"durationMs": 1763.2869999999912,
"styleRecalcs": 32,
"styleRecalcDurationMs": 19.073,
"layouts": 6,
"layoutDurationMs": 0.66,
"taskDurationMs": 388.517,
"heapDeltaBytes": -3616356,
"heapUsedBytes": 45026404,
"domNodes": -209,
"jsHeapTotalBytes": 19787776,
"scriptDurationMs": 26.380999999999993,
"eventListeners": -116,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "dom-widget-clipping",
"durationMs": 553.0369999999891,
"styleRecalcs": 10,
"styleRecalcDurationMs": 7.887000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 364.63800000000003,
"heapDeltaBytes": 9184756,
"heapUsedBytes": 57796416,
"domNodes": 16,
"jsHeapTotalBytes": 15204352,
"scriptDurationMs": 62.84300000000001,
"eventListeners": 2,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "dom-widget-clipping",
"durationMs": 597.9839999999967,
"styleRecalcs": 13,
"styleRecalcDurationMs": 9.859,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 382.014,
"heapDeltaBytes": 9833232,
"heapUsedBytes": 59079480,
"domNodes": 22,
"jsHeapTotalBytes": 15466496,
"scriptDurationMs": 69.464,
"eventListeners": 0,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "large-graph-idle",
"durationMs": 2021.7250000000035,
"styleRecalcs": 9,
"styleRecalcDurationMs": 8.225,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 645.387,
"heapDeltaBytes": 40884840,
"heapUsedBytes": 99724752,
"domNodes": -261,
"jsHeapTotalBytes": 33320960,
"scriptDurationMs": 97.431,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66999999999998,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-idle",
"durationMs": 2040.5000000000086,
"styleRecalcs": 8,
"styleRecalcDurationMs": 6.856999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 521.2520000000001,
"heapDeltaBytes": 8131732,
"heapUsedBytes": 66562004,
"domNodes": -264,
"jsHeapTotalBytes": 290816,
"scriptDurationMs": 80.016,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-pan",
"durationMs": 2115.51399999999,
"styleRecalcs": 70,
"styleRecalcDurationMs": 20.262,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1107.416,
"heapDeltaBytes": 10213508,
"heapUsedBytes": 76164196,
"domNodes": 18,
"jsHeapTotalBytes": 16252928,
"scriptDurationMs": 428.983,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "large-graph-pan",
"durationMs": 2110.7319999999845,
"styleRecalcs": 68,
"styleRecalcDurationMs": 17.317,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 1034.435,
"heapDeltaBytes": 5821400,
"heapUsedBytes": 65661380,
"domNodes": -264,
"jsHeapTotalBytes": -552960,
"scriptDurationMs": 364.045,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333335,
"p95FrameDurationMs": 16.699999999999818
},
{
"name": "large-graph-zoom",
"durationMs": 3246.532000000002,
"styleRecalcs": 65,
"styleRecalcDurationMs": 19.739,
"layouts": 60,
"layoutDurationMs": 8.318000000000001,
"taskDurationMs": 1413.157,
"heapDeltaBytes": 39236448,
"heapUsedBytes": 102143352,
"domNodes": -266,
"jsHeapTotalBytes": 34951168,
"scriptDurationMs": 496.031,
"eventListeners": -127,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "large-graph-zoom",
"durationMs": 3200.885000000028,
"styleRecalcs": 65,
"styleRecalcDurationMs": 18.899,
"layouts": 60,
"layoutDurationMs": 8.202000000000002,
"taskDurationMs": 1352.442,
"heapDeltaBytes": 13110228,
"heapUsedBytes": 73907072,
"domNodes": -268,
"jsHeapTotalBytes": 290816,
"scriptDurationMs": 510.10900000000004,
"eventListeners": -125,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "minimap-idle",
"durationMs": 2068.443000000002,
"styleRecalcs": 10,
"styleRecalcDurationMs": 10.345999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 771.1589999999999,
"heapDeltaBytes": 10920180,
"heapUsedBytes": 76159356,
"domNodes": -259,
"jsHeapTotalBytes": 15609856,
"scriptDurationMs": 115.102,
"eventListeners": -161,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.670000000000012,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "minimap-idle",
"durationMs": 2023.0809999999906,
"styleRecalcs": 8,
"styleRecalcDurationMs": 7.237999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 558.973,
"heapDeltaBytes": 10527088,
"heapUsedBytes": 69165496,
"domNodes": -262,
"jsHeapTotalBytes": 552960,
"scriptDurationMs": 98.465,
"eventListeners": -129,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 552.4110000000064,
"styleRecalcs": 45,
"styleRecalcDurationMs": 10.296,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 370.321,
"heapDeltaBytes": 15460100,
"heapUsedBytes": 64583836,
"domNodes": 16,
"jsHeapTotalBytes": 16777216,
"scriptDurationMs": 126.98800000000001,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 570.968999999991,
"styleRecalcs": 46,
"styleRecalcDurationMs": 11.206999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 381.6120000000001,
"heapDeltaBytes": 14874956,
"heapUsedBytes": 63908524,
"domNodes": 18,
"jsHeapTotalBytes": 17563648,
"scriptDurationMs": 125.994,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2004.7250000000076,
"styleRecalcs": 9,
"styleRecalcDurationMs": 7.674999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 368.649,
"heapDeltaBytes": 22930724,
"heapUsedBytes": 71928728,
"domNodes": 18,
"jsHeapTotalBytes": 14942208,
"scriptDurationMs": 16.645,
"eventListeners": 6,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-idle",
"durationMs": 2006.3650000000166,
"styleRecalcs": 11,
"styleRecalcDurationMs": 10.137,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 431.72599999999994,
"heapDeltaBytes": 1738952,
"heapUsedBytes": 67947168,
"domNodes": 21,
"jsHeapTotalBytes": 19230720,
"scriptDurationMs": 24.514,
"eventListeners": 4,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 2021.8969999999956,
"styleRecalcs": 83,
"styleRecalcDurationMs": 44.337,
"layouts": 16,
"layoutDurationMs": 4.64,
"taskDurationMs": 994.913,
"heapDeltaBytes": 14880588,
"heapUsedBytes": 63956892,
"domNodes": -263,
"jsHeapTotalBytes": 19787776,
"scriptDurationMs": 98.996,
"eventListeners": -131,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1734.863999999959,
"styleRecalcs": 76,
"styleRecalcDurationMs": 38.277,
"layouts": 16,
"layoutDurationMs": 4.920000000000001,
"taskDurationMs": 735.862,
"heapDeltaBytes": -3387732,
"heapUsedBytes": 45581532,
"domNodes": -262,
"jsHeapTotalBytes": 15331328,
"scriptDurationMs": 97.446,
"eventListeners": -133,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.800000000000182
},
{
"name": "subgraph-transition-enter",
"durationMs": 1200.7049999999708,
"styleRecalcs": 16,
"styleRecalcDurationMs": 27.614,
"layouts": 5,
"layoutDurationMs": 11.626000000000001,
"taskDurationMs": 819.6650000000001,
"heapDeltaBytes": 30341864,
"heapUsedBytes": 95443704,
"domNodes": 13513,
"jsHeapTotalBytes": 16515072,
"scriptDurationMs": 36.836999999999996,
"eventListeners": 2529,
"totalBlockingTimeMs": 129,
"frameDurationMs": 16.66333333333332,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "viewport-pan-sweep",
"durationMs": 8466.75799999997,
"styleRecalcs": 251,
"styleRecalcDurationMs": 61.157999999999994,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 4376.782,
"heapDeltaBytes": 80732700,
"heapUsedBytes": 138133308,
"domNodes": -261,
"jsHeapTotalBytes": 72585216,
"scriptDurationMs": 1327.185,
"eventListeners": -155,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.80000000000109
},
{
"name": "viewport-pan-sweep",
"durationMs": 8432.598999999982,
"styleRecalcs": 250,
"styleRecalcDurationMs": 56.534,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 3889.352,
"heapDeltaBytes": 88431088,
"heapUsedBytes": 148745084,
"domNodes": -264,
"jsHeapTotalBytes": 66846720,
"scriptDurationMs": 1158.6940000000002,
"eventListeners": -125,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000728
},
{
"name": "vue-large-graph-idle",
"durationMs": 13570.467000000008,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 13540.163999999999,
"heapDeltaBytes": -43329108,
"heapUsedBytes": 172124232,
"domNodes": -8331,
"jsHeapTotalBytes": 26275840,
"scriptDurationMs": 601.565,
"eventListeners": -16460,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.220000000000073,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-idle",
"durationMs": 12629.33899999996,
"styleRecalcs": 0,
"styleRecalcDurationMs": 0,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 12618.880000000001,
"heapDeltaBytes": -26223660,
"heapUsedBytes": 172735656,
"domNodes": -8331,
"jsHeapTotalBytes": 24702976,
"scriptDurationMs": 618.773,
"eventListeners": -16464,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333237,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 19142.611000000044,
"styleRecalcs": 131,
"styleRecalcDurationMs": 26.466999999999963,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 19120.65,
"heapDeltaBytes": -17069228,
"heapUsedBytes": 259203940,
"domNodes": -8327,
"jsHeapTotalBytes": 4517888,
"scriptDurationMs": 1196.3980000000001,
"eventListeners": -16488,
"totalBlockingTimeMs": 61,
"frameDurationMs": 17.776666666666763,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "vue-large-graph-pan",
"durationMs": 14957.090999999991,
"styleRecalcs": 67,
"styleRecalcDurationMs": 18.542,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 14937.836999999998,
"heapDeltaBytes": -36179092,
"heapUsedBytes": 165368040,
"domNodes": -8331,
"jsHeapTotalBytes": -3608576,
"scriptDurationMs": 882.129,
"eventListeners": -16458,
"totalBlockingTimeMs": 0,
"frameDurationMs": 17.223333333333358,
"p95FrameDurationMs": 16.799999999999272
},
{
"name": "workflow-execution",
"durationMs": 458.4509999999682,
"styleRecalcs": 17,
"styleRecalcDurationMs": 25.459,
"layouts": 5,
"layoutDurationMs": 1.6229999999999998,
"taskDurationMs": 132.81,
"heapDeltaBytes": 5577496,
"heapUsedBytes": 59729700,
"domNodes": 168,
"jsHeapTotalBytes": 1048576,
"scriptDurationMs": 26.758000000000003,
"eventListeners": 69,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.666666666666668,
"p95FrameDurationMs": 16.700000000000273
},
{
"name": "workflow-execution",
"durationMs": 451.5450000000101,
"styleRecalcs": 18,
"styleRecalcDurationMs": 23.732,
"layouts": 4,
"layoutDurationMs": 1.091,
"taskDurationMs": 113.26599999999998,
"heapDeltaBytes": 5065908,
"heapUsedBytes": 55202552,
"domNodes": 159,
"jsHeapTotalBytes": 262144,
"scriptDurationMs": 22.785,
"eventListeners": 71,
"totalBlockingTimeMs": 0,
"frameDurationMs": 16.663333333333338,
"p95FrameDurationMs": 16.700000000000273
}
]
} |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/litegraph/src/subgraph/SubgraphInputNode.ts (1)
229-235: ⚡ Quick winEvent emission completes disconnect reactivity.
The disconnect path now emits
node:slot-links:changedwithconnected: false, mirroring the connect logic. Note that the event fires afteronConnectionsChangehere, whereas inSubgraphInput.connect()it fires before. Verify this ordering difference is intentional for the different lifecycle phases.🔧 Optional: Use property shorthand
subgraph.trigger('node:slot-links:changed', { nodeId: node.id, slotType: NodeSlotType.INPUT, - slotIndex: slotIndex, + slotIndex, connected: false, linkId: link.id })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/litegraph/src/subgraph/SubgraphInputNode.ts` around lines 229 - 235, The event emission in SubgraphInputNode.disconnect currently calls subgraph.trigger('node:slot-links:changed', { nodeId, slotType, slotIndex, connected: false, linkId }) after onConnectionsChange, while SubgraphInput.connect fires that same event before onConnectionsChange; confirm whether this ordering difference is intentional for lifecycle reasons, and if not move the trigger in SubgraphInputNode.disconnect to execute before onConnectionsChange to match SubgraphInput.connect (or add a comment documenting the intentional ordering). Also consider using property shorthand for the payload object keys (nodeId, slotType, slotIndex, linkId) to simplify the call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@browser_tests/fixtures/helpers/SubgraphHelper.ts`:
- Around line 245-254: getInputBounds assumes a subgraph and input node always
exist and can throw opaque errors; add explicit guards inside getInputBounds
(and inside the page.evaluate callback) to check that app, app.canvas,
app.canvas.graph is a Subgraph, and graph.inputNode exists before calling
convertOffsetToCanvas or accessing size/pos, and return a sensible fallback
(e.g., null or a rejected Promise with a clear error) if preconditions fail;
specifically update the getInputBounds function and the evaluate callback that
references app!.canvas.graph, inputNode, and
app!.canvas.ds.convertOffsetToCanvas to validate those symbols and surface a
clear error message instead of letting a runtime exception propagate.
In `@src/renderer/extensions/vueNodes/composables/useSlotLinkInteraction.ts`:
- Around line 414-415: In useSlotLinkInteraction, avoid capturing app.canvas and
node at composable creation; instead re-resolve canvas via app.canvas and the
node via canvas.graph.getNodeById(nodeId) inside the pointer handlers (e.g., the
pointer move/drag and pointer down/up callbacks referenced around the existing
canvas/node usage) and guard each use with null/undefined checks so you never
operate on a stale or unready canvas/graph/node; update the usages at the spots
that previously read the top-level const canvas and const node (and the
subsequent block at the later references) to resolve them lazily inside the
handler before using them.
---
Nitpick comments:
In `@src/lib/litegraph/src/subgraph/SubgraphInputNode.ts`:
- Around line 229-235: The event emission in SubgraphInputNode.disconnect
currently calls subgraph.trigger('node:slot-links:changed', { nodeId, slotType,
slotIndex, connected: false, linkId }) after onConnectionsChange, while
SubgraphInput.connect fires that same event before onConnectionsChange; confirm
whether this ordering difference is intentional for lifecycle reasons, and if
not move the trigger in SubgraphInputNode.disconnect to execute before
onConnectionsChange to match SubgraphInput.connect (or add a comment documenting
the intentional ordering). Also consider using property shorthand for the
payload object keys (nodeId, slotType, slotIndex, linkId) to simplify the call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fcddccaa-ac35-4fba-91a2-8e6e122bea01
📒 Files selected for processing (7)
browser_tests/fixtures/helpers/SubgraphHelper.tsbrowser_tests/tests/subgraph/subgraphSlots.spec.tssrc/lib/litegraph/src/LGraphCanvas.tssrc/lib/litegraph/src/subgraph/SubgraphInput.tssrc/lib/litegraph/src/subgraph/SubgraphInputNode.tssrc/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/renderer/extensions/vueNodes/composables/useSlotLinkInteraction.ts
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #12281 +/- ##
===========================================
- Coverage 73.78% 59.80% -13.99%
===========================================
Files 1520 1411 -109
Lines 84941 72136 -12805
Branches 22393 20017 -2376
===========================================
- Hits 62674 43141 -19533
- Misses 21455 28522 +7067
+ Partials 812 473 -339
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1012 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Updating Playwright Expectations |
96ee7ac to
23c5d31
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
browser_tests/tests/subgraph/subgraphSlots.spec.ts (1)
676-676: ⚡ Quick winPrefer a more stable selector over DOM traversal.
Using
locator('../..')to navigate up the DOM tree is brittle and will break if the component's DOM structure changes. Consider using a data attribute (e.g.,data-testid) on the slot container or querying from a more stable parent reference.♻️ Suggested refactor
If the slot container has a test ID:
-const slotParent = stepsSlot.locator('../..') +const slotParent = stepsSlot.locator('[data-testid="slot-container"]')Or query from the node level:
-const slotParent = stepsSlot.locator('../..') +const slotParent = ksampler.locator('[data-slot-key="steps"]').locator('..')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@browser_tests/tests/subgraph/subgraphSlots.spec.ts` at line 676, The test creates slotParent by brittle DOM traversal using stepsSlot.locator('../..'); instead, add or use a stable selector on the slot container (e.g., data-testid="slot-parent") and update the assignment of slotParent to query that stable selector (replace stepsSlot.locator('../..') with a locator that targets the data-testid or a stable parent locator), or if adding attributes isn't possible, use an ancestor selector that targets a stable attribute/class (e.g., stepsSlot.locator('xpath=ancestor::*[`@data-testid`="slot-parent"]')) so tests rely on stable identifiers rather than DOM-tree navigation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@browser_tests/tests/subgraph/subgraphSlots.spec.ts`:
- Line 676: The test creates slotParent by brittle DOM traversal using
stepsSlot.locator('../..'); instead, add or use a stable selector on the slot
container (e.g., data-testid="slot-parent") and update the assignment of
slotParent to query that stable selector (replace stepsSlot.locator('../..')
with a locator that targets the data-testid or a stable parent locator), or if
adding attributes isn't possible, use an ancestor selector that targets a stable
attribute/class (e.g.,
stepsSlot.locator('xpath=ancestor::*[`@data-testid`="slot-parent"]')) so tests
rely on stable identifiers rather than DOM-tree navigation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b088fee7-aa05-4323-a07c-3d0f382cbee6
⛔ Files ignored due to path filters (1)
browser_tests/tests/subgraph/subgraphSlots.spec.ts-snapshots/vue-io-highlight-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (1)
browser_tests/tests/subgraph/subgraphSlots.spec.ts
| } | ||
| const isSubgraphIOLink = | ||
| linkConnector.isConnecting && | ||
| Number(linkConnector.renderLinks.at(0)?.node?.id ?? 0) < 0 |
There was a problem hiding this comment.
Can we use the subgraph IO constants instead of < 0, clearer and safer
There was a problem hiding this comment.
Not sure how many other uses in the code do this, but may be worth extracting this as a function
There was a problem hiding this comment.
I agree that something reusable would be better here.
I'm leaning towards a prop on RenderLink since the extra resolution steps of node -> id -> is a specific id feels a little brittle. In particular, there's been some recent attempts to make ids globally unique which have completely forgotten about subgraph nodes.
|
|
||
| autoPan?.updatePointer(event.clientX, event.clientY) | ||
|
|
||
| if (canvas.subgraph && node) { |
There was a problem hiding this comment.
Can we also guard on canvas.linkConnector.isConnecting or potentially the direction we are linking to not need to call both input & output
There was a problem hiding this comment.
Can we also guard on canvas.linkConnector.isConnecting
The 'handlePointerMove' function this is inside only fires while connecting links.
potentially the direction we are linking to not need to call both input & output
I had intentionally included both for parity with the litegraph implementation, as it will also dim links on the other IONode.
From further review, the litegraph implementations were half-baked and did not filter on input/output. I have corrected this as well.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/litegraph/src/subgraph/SubgraphOutput.ts (1)
143-144: ⚡ Quick winConsider introducing type guards for clearer slot direction narrowing.
The current implementation using
'links' in fromSlotcorrectly identifies output slots at runtime (since onlyINodeOutputSlothas thelinksproperty), but TypeScript doesn't narrow the type. While the code functions correctly, adding dedicated type guards would improve type safety and reusability.✨ Suggested type guards in subgraphUtils
Add to
src/lib/litegraph/src/subgraph/subgraphUtils.ts:export function isNodeInputSlot( slot: INodeInputSlot | INodeOutputSlot ): slot is INodeInputSlot { return 'link_id' in slot } export function isNodeOutputSlot( slot: INodeInputSlot | INodeOutputSlot ): slot is INodeOutputSlot { return 'links' in slot }Then update this file:
-import { isNodeSlot, isSubgraphInput } from './subgraphUtils' +import { isNodeSlot, isNodeOutputSlot, isSubgraphInput } from './subgraphUtils'- if (isNodeSlot(fromSlot) && 'links' in fromSlot) { + if (isNodeSlot(fromSlot) && isNodeOutputSlot(fromSlot)) { return LiteGraph.isValidConnection(fromSlot.type, this.type) }A similar change in
SubgraphInput.tswithisNodeInputSlotwould complete the pattern.Based on learnings, prefer user-defined type guards over inline type assertions when narrowing union types in litegraph code to avoid casts, improve reuse, and enhance type safety.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/litegraph/src/subgraph/SubgraphOutput.ts` around lines 143 - 144, Replace the inline runtime check "'links' in fromSlot" in SubgraphOutput.ts with a user-defined type guard: add isNodeInputSlot and isNodeOutputSlot to src/lib/litegraph/src/subgraph/subgraphUtils.ts (signatures that narrow INodeInputSlot vs INodeOutputSlot using 'link_id' and 'links' respectively) and then use isNodeOutputSlot(fromSlot) in the SubgraphOutput.isValidConnection logic (and similarly update SubgraphInput to use isNodeInputSlot) so TypeScript properly narrows the slot union without casts and the guards are reusable across the subgraph code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/litegraph/src/subgraph/SubgraphOutput.ts`:
- Around line 143-144: Replace the inline runtime check "'links' in fromSlot" in
SubgraphOutput.ts with a user-defined type guard: add isNodeInputSlot and
isNodeOutputSlot to src/lib/litegraph/src/subgraph/subgraphUtils.ts (signatures
that narrow INodeInputSlot vs INodeOutputSlot using 'link_id' and 'links'
respectively) and then use isNodeOutputSlot(fromSlot) in the
SubgraphOutput.isValidConnection logic (and similarly update SubgraphInput to
use isNodeInputSlot) so TypeScript properly narrows the slot union without casts
and the guards are reusable across the subgraph code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3f2051c1-689f-48fc-9d3a-9bd778f50f4e
📒 Files selected for processing (6)
src/lib/litegraph/src/LGraphCanvas.tssrc/lib/litegraph/src/canvas/RenderLink.tssrc/lib/litegraph/src/canvas/ToInputFromIoNodeLink.tssrc/lib/litegraph/src/canvas/ToOutputFromIoNodeLink.tssrc/lib/litegraph/src/subgraph/SubgraphInput.tssrc/lib/litegraph/src/subgraph/SubgraphOutput.ts
✅ Files skipped from review due to trivial changes (2)
- src/lib/litegraph/src/canvas/ToOutputFromIoNodeLink.ts
- src/lib/litegraph/src/canvas/RenderLink.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/litegraph/src/subgraph/SubgraphInput.ts
|
See also #9838 which takes a very different approach to solving the same issues. It gets cleaner handling for links from subgraph IO snapping to vue nodes, but has a bit more code duplication than I would like and seemed to be less dependable on the other bugs. |
|
@AustinMroz Successfully backported to #12378 |
|
@AustinMroz Successfully backported to #12379 |
Backport of #12281 to `core/1.44` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12378-backport-core-1-44-Subgraph-io-fixes-3666d73d365081b79ccfc86094cb0f24) by [Unito](https://www.unito.io) --------- Co-authored-by: AustinMroz <austin@comfy.org>
Backport of #12281 to `cloud/1.44` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12379-backport-cloud-1-44-Subgraph-io-fixes-3666d73d36508166b709c77b71eb013f) by [Unito](https://www.unito.io) --------- Co-authored-by: AustinMroz <austin@comfy.org>
Fixes 3 different bugs when making links to and from subgraph IO from vue nodes
Resolves FE-561
┆Issue is synchronized with this Notion page by Unito